HBASE-30323 [RSGroup] Forward-port HBASE-22658 to branch-2 - #8547

How about adding an IT test case for scenario when all servers in test RS group go down and fallback config is enabled (hbase.rsgroup.fallback.enable = true), then regions from test RS group do move to default RS group servers on calling RegionMover.unload().

There is already test written for that, 'TestRSGroupsFallback'.

The PR is ready to merge. We just need to co-ordinate when to merge this, I think it will be better to merge this after your fix on master branch is first merged and backported to branch-2 and others. Please let me know if there is any concern, I am fine either way.

@virajjasani Copilot found at least one issue (the first one) that should be resolved before merge.

The move of RSGroupAdmin.proto from hbase-rsgroup to hbase-protocol is not necessary and the way you've left the state of the POM in hbase-rsgroup the protobuf compilation still happens there but now there's no input files. I guess it works?? but is messy.

We move RSGroups out of the separate module into core in HBase 3 and up, so there's that, but the way we hedged RSGroups in branch-2 puts everything else in hbase-rsgroup. It's fine and even customary to have protos outside of hbase-protos. We have the protos for REST broken out into hbase-rest, for example, so this is maybe a smell.

This is not a dealbreaker, but there is an alternative that does not require it. Consider just using RSGroupTableAccessor from hbase-client. Skip the coprocessor RPC and read the hbase:rsgroup system table directly via the accessor that already exists in hbase-client. Something like:

private RSGroupInfo getRSGroupInfo(String host, int port) throws IOException {
  if (!RSGroupTableAccessor.isRSGroupsEnabled(conn)) {
    return null;
  }
  Address addr = Address.fromParts(host, port);
  for (RSGroupInfo info : RSGroupTableAccessor.getAllRSGroupInfo(conn)) {
    if (info.containsServer(addr)) {
      return info;
    }
  }
  return null;
}

I also like this approach better because the invocation of the coprocessor is brittle. It works today but any subclass or wrapper would break it, if someone is extending rsgroups and implementing their own enhanced coprocessors. Unlikely but not impossible.

Pushed 3dcff72 addressing the remaining review feedback:

@apurtell's suggestion — switched getRSGroupInfo() to read hbase:rsgroup directly via RSGroupTableAccessor (from hbase-client) instead of the RSGroupAdminEndpoint coprocessor RPC. This let me move RSGroupAdmin.proto back to hbase-rsgroup, so hbase-protocol no longer carries it — the messy proto relocation is gone.

Copilot findings:

• getRSGroupInfo() now throws IOException instead of returning null when RSGroups are enabled but the server can't be matched to any group. Previously that case fell through to treating every online server as a valid destination, which defeats RSGroup isolation during decommission.
• testUnloadRegionsRespectsRSGroup now picks the decommission target by checking which rsservers member actually hosts a TABLE_NAME region, rather than assuming rsservers.get(0). moveTableRegionsToGroup() places regions via randomAssignment(), so the old assumption could let the test pass without exercising the move/filter path at all.
• Fixed the Javadoc on testUnloadDefaultGroupServerWithRSGroupEnabled — it said regions "spread across all available servers (not filtered)" but the test actually asserts destinations are filtered to the default group.

Verified: TestRegionMoverFilterRSGroupServers (3/3) and TestRegionMoverWithRSGroupEnable (2/2) pass, checkstyle and spotbugs clean on both hbase-server and hbase-rsgroup.

The move of RSGroupAdmin.proto from hbase-rsgroup to hbase-protocol is not necessary and the way you've left the state of the POM in hbase-rsgroup the protobuf compilation still happens there but now there's no input files. I guess it works?? but is messy.

We move RSGroups out of the separate module into core in HBase 3 and up, so there's that, but the way we hedged RSGroups in branch-2 puts everything else in hbase-rsgroup. It's fine and even customary to have protos outside of hbase-protos. We have the protos for REST broken out into hbase-rest, for example, so this is maybe a smell.

This is not a dealbreaker, but there is an alternative that does not require it. Consider just using RSGroupTableAccessor from hbase-client. Skip the coprocessor RPC and read the hbase:rsgroup system table directly via the accessor that already exists in hbase-client. Something like:

private RSGroupInfo getRSGroupInfo(String host, int port) throws IOException {
  if (!RSGroupTableAccessor.isRSGroupsEnabled(conn)) {
    return null;
  }
  Address addr = Address.fromParts(host, port);
  for (RSGroupInfo info : RSGroupTableAccessor.getAllRSGroupInfo(conn)) {
    if (info.containsServer(addr)) {
      return info;
    }
  }
  return null;
}

I also like this approach better because the invocation of the coprocessor is brittle. It works today but any subclass or wrapper would break it, if someone is extending rsgroups and implementing their own enhanced coprocessors. Unlikely but not impossible.

Thanks a lot. @apurtell . I missed this.

I checked both are clean cherry-pick to 2.5 and 2.6.

Triggered github workflow, please keep an eye if we get any non-flaky test failures